Repository navigation
fix(db): reconcile shared-state models with Supabase migrations - #2624
Draft
groupthinking wants to merge 3 commits into
Draft
groupthinking wants to merge 3 commits into
groupthinking wants to merge 3 commits into
Conversation
- Add apps/web/supabase/migrations/20261109062000_create_service_role_shared_state_tables.sql for shared_sessions, shared_timeline_events and shared_a2a_messages, matching the SQLAlchemy tables exactly; idempotent; RLS enabled and forced, service_role only. - SharedSQLStateStore: create_all only on SQLite; on Postgres verify tables/columns and raise SharedStateSchemaError naming the migration. - Add docs/schema_inventory.json and update docs/MIGRATION_TRUTH.md. - Add tests/unit/test_schema_inventory.py (migration vs model drift, forced RLS) and tests/unit/test_shared_sql_state.py. Refs #2169
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
github-actions
Bot
added
documentation
Contributor
🔍 PR Validation |
Contributor
|
Please add a See governance: #898 |
Contributor
✅ E2E Test Results: ALL TESTS PASSED
Test Output |
Copilot started work to fix failing Actions workflows on behalf of
groupthinking
November 9, 2026 06:30
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The schema check accepts unhardened legacy PostgreSQL tables, allowing initialization without required RLS and restricted grants despite the documented fail-closed guarantee.
3 open issues
What changed in this PR
This PR establishes checked-in migration ownership for shared session and agent-message storage in EventRelay, UVAI’s internal runtime, addressing schema drift tracked in #2169.
Changes:
- Adds a Supabase migration for three shared-state tables, with forced row-level security and service-role-only grants.
- Keeps automatic table creation on SQLite while checking table and column presence on other databases.
- Adds schema inventory and persistence tests, and documents ownership, access requirements, unresolved sources, and manual deployment steps.
| File | Description |
|---|---|
tests/unit/test_shared_sql_state.py |
Tests SQLite persistence, schema checks, and opt-in PostgreSQL behavior. |
tests/unit/test_schema_inventory.py |
Checks inventory paths, model/migration alignment, and security declarations. |
src/youtube_extension/services/shared_sql_state.py |
Restricts automatic schema creation to SQLite and adds validation elsewhere. |
docs/schema_inventory.json |
Maps tables to migrations and records unreconciled SQL sources. |
docs/MIGRATION_TRUTH.md |
Documents schema ownership, access rules, and deployment requirements. |
apps/web/supabase/migrations/20261109062000_create_service_role_shared_state_tables.sql |
Defines shared-state tables, indexes, forced row-level security, and restricted grants. |
Review effort: Max
groupthinking
marked this pull request as draft
November 13, 2026 01:48
Check public schema, forced RLS, effective client access, server role attributes/grants, serial ACLs, and expected schema-error type. Document plain PostgreSQL prerequisites and complete inventory. Add disposable PostgreSQL acceptance CI and disable this exact branch deployment. Refs #2169
All 13 disposable PostgreSQL 16.2 security/persistence cases passed in run 43347589645 on the unchanged application source. Production migration remains a separate human step. Refs #2169
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Canonical issue
Closes #2664.
Refs #2169 (AUDIT-007). This reconciles shared-state SQL ownership and startup hardening; unmanaged legacy SQL and live migration application remain separate work. The broader audit must remain open.
Result
SharedSQLStateStore creates local SQLite tables and requires migrated PostgreSQL tables. Startup verifies the actual PostgreSQL security state, including legacy tables that already have every expected column:
512a2558f73d6281b6a880548bedf140b7e30292; disable Vercel deployment for this exact review branch in both root and app configuration using git.deploymentEnabled.Verified evidence — November 12, 2026
Current head:
2bd4aa5b1590bcd21a1911c1beecbc4bd2193864, tree88dfebdc81dc3478dd8873b9532b3d5bb2d73895. The final follow-up changes only the migration documentation. Its application source is identical to hosted acceptance head190fc17f7b38a19978094e08aaf3deac5f745c59.Actual PostgreSQL 16.2 acceptance: 13/13 passed. GitHub run 43347589645, job 131630572786 records migration applied twice, dedicated server-role writes and fresh-store readback, rejection of legacy create_all security, disabled/unforced RLS, direct and inherited client grants, client BYPASSRLS and superuser, missing service grants, client sequence privileges, server without BYPASSRLS, and public-schema writes despite a hostile search_path. The runner verified no IP listener, then stopped and deleted its temporary database.
Local Python 3.12.14, pytest 9.2.0, SQLAlchemy 2.1.8:
env -u DATABASE_URL -u EVENTRELAY_SHARED_STATE_DATABASE_URL PYTHONPATH=src python -m pytest tests/unit/test_schema_inventory.py tests/unit/test_shared_sql_state.py tests/unit/test_agent_orchestrator.py tests/unit/test_session_orchestration.py -o addopts='' -o log_cli=false -q101 passed, 1 skipped, exit 0. The skip is the separate environment-gated PostgreSQL unit test; the hosted acceptance above independently used real PostgreSQL. Five initial new hardening regressions failed against the old source. PostgreSQL catalog SQL parses; whitespace and script compile checks pass. Independent review identified schema-shadowing/inherited-ACL gaps; these corrections and the hosted acceptance cover them.
Combined #2624/#2626/#2628 source validation: 208 passed, 1 skipped, exit 0. SharedStateSchemaError degradation was directly checked without database access. A separate combined deployment/classifier regression confirms failed Vercel work preserves GitHub provenance and returns failure; the fully successful positive control remains successful. Local suites retain one pre-existing unawaited-AsyncMock warning.
Remaining gates and production
Keep draft, unmerged and undeployed. Current-head hosted CI/security and the issue-link gate remain active. This does not apply migrations to Supabase or any production database; application of the checked-in migration remains a deliberate authorized human step. The initial local PostgreSQL attempt was blocked by Unix-socket permissions, and was replaced by the successful hosted acceptance above. No production, paid extraction, provider execution or deployment acceptance is claimed.